Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ImageTicker and BefreAfterSlider components #16

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

man-shar
Copy link
Contributor

@man-shar man-shar commented Dec 6, 2021

Adds an ImageTicker component that looks like this: https://www.dropbox.com/s/clzcs8bwzm5lw3o/ImageTicker.mp4?dl=0

Adds a BeforeAfterSlider component that looks like this: https://www.dropbox.com/s/zxs8l3r78ex8yfc/BeforeAfterSlider2.mp4?dl=0

@hobbes7878
Copy link
Member

I bet you thought I'd all but forgotten about this @man-shar! :) Finally got some spare minutes to have a look, tho!

OK, first thing's first, I added a demo of the new component. That way we don't have to have pass around a dropbox video AND folks using the component get some documentation on how to use it.

The syntax to add a demo is a little funky (see the new docs.svx in your component directory) 'cause we use MDsveX, which is basically markdown that you can write Svelte in. It makes your VSCode linter go crazy, but it's perfect for quickly writing up a demo page with minimal fuss.

That step out of the way, I have a few more questions about how you see folks using this component and whether we've hit that sweet spot of building something that can be configured to cover ~80% of the different uses for this kind of slideshow. So for example, this is a play through on an interval. Do we want to ALSO be able control that in different ways? Is a true slideshow, with like... buttons an alternative we want to handle here? Should we externalise what controls the play-through, even, so like, maybe you just pass a prop that says which image should be active, and then it's up to you how you want that to play: buttons, interval, scroll behaviour, whatever. What about different sizes of image? Should we make the image in here reflect our wide/wider/widest/fluid class options so folks can easily break out of the text well? Other layout options with the text we may want to accommodate?

Anyway, this is the kind of stuff I always think about with components. Delicate balance between making a swiss army knife, which does too much, and a spoon, which only really does one thing.

Let me know what you think on those Qs.

In terms of process for these PRs, what I'd love to do is have a step where we review new components across our devs before we adopt them into the library. Doesn't need to be an overly formalised process, but a good gut check with how someone who isn't the principle developer would use a team component. So maybe Minami or Fielding can take a look once we're happy with this one.

@man-shar
Copy link
Contributor Author

man-shar commented Jan 4, 2022

I'm glad someone remembered haha. I'd totally forgotten :P

Your questions:

So for example, this is a play through on an interval. Do we want to ALSO be able control that in different ways? Is a true slideshow, with like... buttons an alternative we want to handle here?

I think button based slideshow is a good idea. Currently all you can control is the autoplay time interval. Are there other possibilities?

EDIT: Thought of one: the slider thing for before/after images. How do you feel about that?

Should we externalise what controls the play-through, even, so like, maybe you just pass a prop that says which image should be active, and then it's up to you how you want that to play: buttons, interval, scroll behaviour, whatever.

Hmm, how do you mean "which image should be active"? So someone passes image3.jpg as a prop and the third image is active when the reader gets to it? And added on to that is a prop that defines if it's autoplay or a button slideshow (the one in the above question basically)?

What about different sizes of image?

Yeah, good idea. Can just pass the class name.

Other layout options with the text we may want to accommodate?

Text position? Top: left, middle right, bottom: left, middle right.

EDIT:

Perhaps one thing to add could be a "start autoplay on scroll" feature. esp if the order is important. say a before/after image. Thoughts?

@man-shar
Copy link
Contributor Author

man-shar commented Jan 5, 2022

Okay, I added the button controlled image container to the docs. Lmk how that looks.
Will work on the before/after slider next + "autoplay on scroll"

EDIT: Thinking more about the before/after. That should perhaps be a separate component. It's too much of a special case for this one to fit in elegantly. Added before/after slider component.

@man-shar man-shar changed the title Add ImageTicker component Add ImageTicker and BefreAfterSlider components Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants